fix broken links and reorganize build from source page #12962

aaronmarkham · 2018-10-24T19:13:08Z

Description

This PR fixes broken links and updates the build from source page.

Preview

http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-12962/2/install/build_from_source.html

Comments

The examples now use cmake. Reviewers please verify that the switches' names didn't change for things like opencv and cuda and cudnn between cmake and make.

roywei · 2018-10-24T21:46:17Z

@aaronmarkham Thanks for the contribution!
@mxnet-label-bot [pr-awaiting-review]

roywei

LGTM, one nit

roywei · 2018-10-24T21:48:23Z

docs/install/build_from_source.md

+
+These might be optional, but they're typically desirable as the extend or enhance MXNet's functionality.
+
+* [OpenCV](http://opencv.org/) - Image Loading and Augmentation. Each operation system has different packages and build from source options for OpenCV. Refer to your OS's link in the [Build Instructions by Operating System](#build-instructions-by-operating-system) section for further instructions.


nit: operating system

Thanks @roywei - fixed.

aaronmarkham · 2018-10-24T22:45:34Z

@lebeg - Are the cmake flags in the examples alright? Should there be some others?

anirudhacharya · 2018-10-26T18:28:29Z

docs/install/build_from_source.md


 ```bash
-make USE_OPENCV=0
+cmake -j USE_CUDA=1 USE_CUDA_PATH=/usr/local/cuda USE_CUDNN=1 USE_MKLDNN=1


we usually give these default build options for user to use while building from source. Should we have a section describing different build options from here - https://github.com/apache/incubator-mxnet/blob/master/make/config.mk explaining what each of those do so that the user can make an informed personalized choice for himself? Provide the default options, but also add this section. Thoughts?

It's interesting that we tend to promote using MKLDNN for better performance and in the CMAKE instructions, they imply that it is turned on by default.
However the config.mk has it turned off.
I thought it was supplied as a submodule and it is on by default. Is the config.mk out of date?

And yes, it should be mentioned what happens when you just run make.
Seems like right now you get:
opencv, openmp (that doesn't work on mac out the box), atlas || apple

Looking this config.mk file actually makes things even more unclear...

@anirudhacharya Can you propose the logic that is followed and the recommended flags per general situation?

There is a conversation here - https://lists.apache.org/thread.html/ca3c858a712a66f4b3443dfb395d84e8fdf2c86e297bcd28a3a36ebd@%3Cdev.mxnet.apache.org%3E on including mkl-dnn in mxnet default build.

I think the recommended flags seem okay for now, the way it is. As and when it is decided to include MKL-DNN in the default build, then the recommended flags can also be updated.

What I was suggesting was to have a section in the documentation that describes the flags already provided in the config.mk file( each environment variable has a comment describing what it does). Please let me know if I need to help in any way here, and I would be glad to.

The flags are good

nswamy · 2018-10-29T05:33:29Z

I am wondering this if the build instructions are tested in CI to make sure we are aware of any breakage and users aren't surprised and frustrated.

nswamy · 2018-10-29T05:35:15Z

can you please share a screenshot on how the install selection will look like?

aaronmarkham · 2018-10-29T18:44:51Z

@nswamy I added a preview link to the main description.

I have a proposal for automated testing of installation instructions. That's all out of scope for this update though. I was just trying to make an incremental improvement to this page's info and add some examples for CMAKE since we didn't have any.

The part I'm still worried about is this config.mk file. It seems to contradict what we're saying everywhere else. To me, it doesn't seem to be in alignment with the math library selection, preference, or recommendations. We don't have to solve it now though. If everything is accurate in what I've put in this PR, then we should go ahead and update the site, and then circle back to update the config if necessary.

ankkhedia · 2018-11-01T22:35:07Z

@nswamy ping!

lupesko · 2018-11-20T01:06:12Z

@aaronmarkham have you tested the cmake flags in the doc?
Anything else the community can help with?

aaronmarkham · 2018-11-20T01:09:00Z

I pulled the flags from the cmake PR. I haven't tested the variations. It would be great if the community provided tested/recommended cmake flags for the doc.

lebeg · 2018-11-20T12:40:36Z

docs/install/build_from_source.md


 ```bash
-make USE_OPENCV=0
+cmake -j USE_CUDA=1 USE_CUDA_PATH=/usr/local/cuda USE_CUDNN=1 USE_MKLDNN=1


The flags are good

nswamy · 2018-11-21T16:16:32Z

docs/install/build_from_source.md

+            * [non-Intel CPUs](#recommended-for-Systems-with-non-Intel-CPUs)
+2. [Install the language API binding(s)](#installing-mxnet-language-bindings) you would like to use for MXNet.
+MXNet's newest and most popular API is Gluon. Gluon is built into the Python binding. If Python isn't your preference, you still have more options. MXNet supports several other language APIs:
+    - [Python (includes Gluon)](../api/python/index.html)


The flow does not seem right. We are taking about Installing API bindings and we redirect the users to API page, we should redirect them to the language install page instead.

nswamy · 2018-11-21T16:17:23Z

I had offline discussion with @aaronmarkham and expressed a concern, I am noting here that we suggest to use make in some places, now we seem to be making this decision to advise our users to use CMake instead. I am not saying this is bad, but that we should make it consistent across.

For example Scala still uses make, if we start migrating to CMake and any new feature or flag is introduced in cmake files and forget to maintain in both the places, it will lead to inconsistency in performance/features, etc.,
Should we have discussion on dev@ and make a concerted effort to move to CMake ? WDYT?

nswamy · 2018-11-21T16:17:42Z

@lebeg is that the command we run in CI?

lebeg · 2018-11-21T16:47:23Z

@nswamy we had already discussions about this and moving to cmake exclusively is the way forward. At the time we are running both make and cmake in CI: runtime_functions.sh

lebeg · 2018-11-21T16:48:35Z

Here is a link to an old discussion: #8702

lebeg · 2018-11-21T16:50:09Z

Discussion on @dev

nswamy · 2018-11-21T17:02:25Z

Thanks for sharing the links. I agree we should move to cmake still doesn't address the inconsistency that can arise, I am assuming you'll start a separate effort to deprecate Make.

aaronmarkham requested a review from szha as a code owner October 24, 2018 19:13

roywei reviewed Oct 24, 2018

View reviewed changes

marcoabreu added the pr-awaiting-review PR is waiting for code review label Oct 24, 2018

fix broken links and reorganize build from source page

168dc0a

aaronmarkham force-pushed the broken_links_20181024 branch from 387c51a to 168dc0a Compare October 24, 2018 22:43

anirudhacharya reviewed Oct 26, 2018

View reviewed changes

aaronmarkham mentioned this pull request Oct 29, 2018

Improve the Clojure Package README to Make it Easier to Get Started #12881

Merged

2 tasks

Merge branch 'master' into broken_links_20181024

6fc4166

lebeg approved these changes Nov 20, 2018

View reviewed changes

nswamy reviewed Nov 21, 2018

View reviewed changes

nswamy merged commit f6317c9 into apache:master Nov 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix broken links and reorganize build from source page #12962

fix broken links and reorganize build from source page #12962

aaronmarkham commented Oct 24, 2018 •

edited

Loading

roywei commented Oct 24, 2018

roywei left a comment

roywei Oct 24, 2018

aaronmarkham Oct 24, 2018

aaronmarkham commented Oct 24, 2018

anirudhacharya Oct 26, 2018

aaronmarkham Oct 26, 2018

anirudhacharya Oct 29, 2018

lebeg Nov 20, 2018

nswamy commented Oct 29, 2018

nswamy commented Oct 29, 2018

aaronmarkham commented Oct 29, 2018

ankkhedia commented Nov 1, 2018

lupesko commented Nov 20, 2018

aaronmarkham commented Nov 20, 2018

lebeg Nov 20, 2018

nswamy Nov 21, 2018

nswamy commented Nov 21, 2018

nswamy commented Nov 21, 2018

lebeg commented Nov 21, 2018

lebeg commented Nov 21, 2018

lebeg commented Nov 21, 2018

nswamy commented Nov 21, 2018


		These might be optional, but they're typically desirable as the extend or enhance MXNet's functionality.

		* [OpenCV](http://opencv.org/) - Image Loading and Augmentation. Each operation system has different packages and build from source options for OpenCV. Refer to your OS's link in the [Build Instructions by Operating System](#build-instructions-by-operating-system) section for further instructions.

fix broken links and reorganize build from source page #12962

fix broken links and reorganize build from source page #12962

Conversation

aaronmarkham commented Oct 24, 2018 • edited Loading

Description

Preview

Comments

roywei commented Oct 24, 2018

roywei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronmarkham commented Oct 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nswamy commented Oct 29, 2018

nswamy commented Oct 29, 2018

aaronmarkham commented Oct 29, 2018

ankkhedia commented Nov 1, 2018

lupesko commented Nov 20, 2018

aaronmarkham commented Nov 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nswamy commented Nov 21, 2018

nswamy commented Nov 21, 2018

lebeg commented Nov 21, 2018

lebeg commented Nov 21, 2018

lebeg commented Nov 21, 2018

nswamy commented Nov 21, 2018

aaronmarkham commented Oct 24, 2018 •

edited

Loading